tests: Enable MCUBoot test on additional platforms #51448

danieldegrasse · 2022-10-19T14:50:08Z

This is an initial attempt to enable testing MCUBoot using twister filtering on a broader variety of platforms, as a follow up to #49552. The test targeted here is tests/boot/test_mcuboot

Introduction

The core issue with filtering tests for MCUBoot using twister is that twister must use the devicetree and KConfig outputs from CMake's configuration step to filter test cases. However, MCUBoot defines an application overlay with the following contents:

/ {
     chosen {
          zephyr,code-partition = &boot_partition;
     };
};

This overlay is added early in the build process, so it cannot be disabled based on a KConfig or devicetree property. Therefore, we must somehow only attempt to configure MCUBoot using CMake if a boot_partition is present in the base devicetree for the board.

Approach

Currently, the approach I have chosen is to use sysbuild for this purpose. By default, sysbuild only configures a small set of KConfig values, and does not parse devicetree at all. However, by changing the included Zephyr modules we can enable devicetree parsing using sysbuild. In addition, by rewriting the included board's defconfig at build time (to use the SB_CONFIG Kconfig prefix), a broader set of KConfig values can be made available as dependencies for symbols in Kconfig.sysbuild. This behavior is currently controlled by passing -DSB_FULL_CONFIG to CMake on the commandline.

With -DSB_FULL_CONFIG=y, KConfig.sysbuild can dynamically decide to skip configuring MCUBoot, allowing this sample to complete CMake for boards that do not support MCUBoot. From here, the test can be filtered normally using twister.

Status of the PR

I have run twister against all in tree boards using this PR, targeting the tests/boot/test_mcuboot sample. Currently, twister fails to build this sample for 16 boards which I believe should support MCUBoot (based on their devicetrees), but do not currently:

bl652_dvk failed: Build failure
bl654_usb failed: Build failure
thingy52_nrf52832 failed: Build failure
nrf5340dk_nrf5340_cpuapp_ns failed: Build failure
olimex_lora_stm32wl_devkit failed: Build failure
pinetime_devkit0 failed: Build failure
arduino_nicla_sense_me failed: Build failure
ruuvi_ruuvitag failed: Build failure
degu_evk failed: Build failure
thingy53_nrf5340_cpuapp failed: Cmake build failure
thingy53_nrf5340_cpuapp_ns failed: Build failure
nrf52840dongle_nrf52840 failed: Build failure
nrf9160dk_nrf9160_ns failed: Build failure
nucleo_g0b1re failed: Build failure
b_u585i_iot02a_ns failed: Build failure
bt610 failed: Build failure

I'd appreciate any feedback on if these board should support MCUBoot and are currently broken, or if they should not support it (and if that is the case, what filtering criteria could be used to determine this)

I have not performed any on device testing using twister with this PR, at this time. I'd appreciate community feedback as we work to narrow down which boards actually are capable of supporting MCUBoot.

Issues to address

I am not entirely happy with the requirement to pass -DSB_FULL_CONFIG=y at the commandline to enable proper filtering for this test, but given how early in the CMake configuration Zephyr modules must be included, I was not sure where else to do so. @tejlmand, would appreciate any feedback here, or on the broader idea of SB_FULL_CONFIG
Currently, this test filters out any ESP32 SOCs unconditionally. As far as I can tell, these SOCs use MCUBoot, but do not support upstream MCUBoot within Zephyr. @sylvioalves, could you clarify if these SOCs should be tested with this sample?

…uild Introduce SB_FULL_CONFIG, to enable systembuild to build zephyr.dts and a full output KConfig file. This option is experimental, and is intended to aid in filtering tests with sysbuild. Setting this flag with -DSB_FULL_CONFIG=y at the command line will enable the following behavior: - "Kconfig.zephyr" must now be sourced in Kconfig.sysbuild, instead of "share/sysbuild/Konfig" - All board configuration settings for a barebones (ie hello_world) build will be available in Kconfig.sysbuild and sysbuild.cmake for the application - All DTS filtering functions will work within Kconfig.sysbuild - A barebones .config and zephyr.dts file will be generated for the root sysbuild build, as well as each sysbuild application Signed-off-by: Daniel DeGrasse <[email protected]>

BOOTLOADER_MCUBOOT selects DT_USE_CODE_PARTITION, which itself depends on FLASH_HAS_LOAD_OFFSET. Remove the dependency on BOOTLOADER_MCUBOOT from FLASH_HAS_LOAD_OFFSET, so it can be used as a filtering criteria to enable mcuboot support Signed-off-by: Daniel DeGrasse <[email protected]>

Remove default y when BOOTLOADER_MCUBOOT is enabled from NORDIC_QSPI_NOR, as this creates a circular dependency when BOOTLOADER_MCUBOOT depends on FLASH_HAS_DRIVER_ENABLED. NORDIC_QSPI_NOR will be enabled when flash drivers are enabled via FLASH=y, so mcuboot support will still function. Signed-off-by: Daniel DeGrasse <[email protected]>

Update MCUBoot test to use twister filtering, using SB_FULL_CONFIG. The use of SB_FULL_CONFIG allows Kconfig.sysbuild to skip building MCUBoot if the platform will not support it, so that cmake configuration can still succeed. This enables twister to filter this sample, so it can be run on additional supported platforms. Signed-off-by: Daniel DeGrasse <[email protected]>

boards/arm/mg100/Kconfig.defconfig

rerickson1 · 2022-10-19T16:08:09Z

bl652_dvk, bl654_usb, and bt610 boards are failing because they are defaulting something (I2C, etc...) which requires multithreading to be turned on explicitly when building the mcuboot image.

danieldegrasse · 2022-10-28T14:17:58Z

bl652_dvk, bl654_usb, and bt610 boards are failing because they are defaulting something (I2C, etc...) which requires multithreading to be turned on explicitly when building the mcuboot image.

@rerickson1 should these boards be enabled for this test then? If not, how can we filter them out? I noticed that bl652_dvk had issues linking with this sample because the swapped_app target uses LMA adjustment, and it appears that enabling SEGGER_RTT is what causes the LMA adjustment to fail. I can add that to the filter to skip this board, but I'm not sure what filter can be used to skip the other two boards.

rerickson1 · 2022-10-28T14:54:30Z

I would prefer these boards be enabled to work with MCUboot as we use MCUboot when building products based off these modules.

Simplest way to solve would be board overlays for the test. Is that acceptable?

str4t0m · 2022-10-28T16:41:17Z

nucleo_g0b1re, olimex_lora_stm32wl_devkit, b_u585i_iot02a_ns all don't define a scratch partition, but mcuboot still defaults to using the scratch algorithm. If the booloader would be built with CONFIG_BOOT_PREFER_SWAP_MOVE they should be able to build. I quickly tested this for the nucleo_g0b1re with success.
Maybe this could even be enabled per default for STM32 targets in the mcuboot repo, as is already the case for SOC_FAMILY_NRF? What are you thoughts about this @erwango?

tejlmand · 2022-10-31T10:16:14Z

I am not entirely happy with the requirement to pass -DSB_FULL_CONFIG=y at the commandline to enable proper filtering for this test, but given how early in the CMake configuration Zephyr modules must be included, I was not sure where else to do so. @tejlmand, would appreciate any feedback here, or on the broader idea of SB_FULL_CONFIG

This is where we should use package helper, and not start creating stuff in sysbuild for such purpose.

For example one may run just dts without doing a full CMake build as:

$ cmake -DBOARD=nrf52840dk_nrf52840 -DMODULES=dts -S tests/boot/test_mcuboot/ -Bbuild -P cmake/package_helper.cmake
Loading Zephyr module(s) (Zephyr workspace): zephyr_default:dts
...
$ grep boot_partition build/zephyr/zephyr.dts
                                        boot_partition: partition@0 {
$

and for another board:

$ cmake -DBOARD=pico_pi_m4 -DMODULES=dts -S tests/boot/test_mcuboot/ -Bbuild -P cmake/package_helper.cmake
Loading Zephyr module(s) (Zephyr workspace): zephyr_default:dts
...
$ grep boot_partition build/zephyr/zephyr.dts
$

as seen, first board has a boot_partition, second board haven't got one.

So twister should run the package helper, and based on outcome from that decide whether or not to run sysbuild on the sample for the given board.

tejlmand

We should update twister to use package_helper before proceeding with this.

It was exactly for such use-cases I created package helper in first place.

danieldegrasse · 2022-11-01T14:30:17Z

I would prefer these boards be enabled to work with MCUboot as we use MCUboot when building products based off these modules.

Simplest way to solve would be board overlays for the test. Is that acceptable?

@rerickson1 I have no problem with this. Twister will filter on the "main" sysbuild application's output devicetree and KConfig settings. What overlays would be required to enable these boards?

danieldegrasse · 2022-11-01T14:37:05Z

We should update twister to use package_helper before proceeding with this.

It was exactly for such use-cases I created package helper in first place.

@tejlmand Ok, I will look into adding support for package helper to twister when time permits. I am assuming that we'll want to enable package helper globally. Are there limitations of the package helper scripts that would prevent this, and require package helpers to be opt-in?

rerickson1 · 2022-11-01T14:52:44Z

@rerickson1 I have no problem with this. Twister will filter on the "main" sysbuild application's output devicetree and KConfig settings. What overlays would be required to enable these boards?

For bl654_usb and bt610, CONFIG_MULTITHREADING=y is needed when building the mcuboot image.
I do not see a failure for bl652_dvk.

tejlmand · 2022-11-03T07:22:23Z

I am assuming that we'll want to enable package helper globally. Are there limitations of the package helper scripts that would prevent this, and require package helpers to be opt-in?

yes, that was always the plan, but seems there has not been time for this work, and without a hard pressure I guess it has been down on the list.

It should indeed be enabled globally.
@PerMac maybe you could support here ?

PerMac · 2022-11-14T14:06:26Z

@tejlmand @danieldegrasse I think the support for packed helper has to be global. I think the runner workflow can be modified in a way, that if keyword filter: is present in a given configuration yaml, a package helper should first generate dts and kconfigs and twister verify them. Then, if configs are supported, a normal twister workflow can be executed. I think such workflow can reduce the total time spend by twister on testing bigger scopes. Even though there will be an extra step, it will save time since full build won't be required to evaluate a config.

sylvioalves · 2022-11-22T14:56:33Z

@danieldegrasse ESP32 boards should be filtered out as it can't be built in Zephyr's environment yet.

github-actions · 2023-01-22T00:30:04Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

cfriedt · 2023-01-22T21:00:39Z

@danieldegrasse ESP32 boards should be filtered out as it can't be built in Zephyr's environment yet.

@sylvioalves - is that still the case?

sylvioalves · 2023-01-22T22:09:04Z

@danieldegrasse ESP32 boards should be filtered out as it can't be built in Zephyr's environment yet.

@sylvioalves - is that still the case?

Yes, for now. We have been working on it.

PerMac · 2023-01-24T08:31:16Z

I've opened a PR which adds the package helper support in twister to move us forward with mcuboot testing. #53499 It adds possibility to filter only on dts without the need to execute (and possibly fail) kconfigs and further steps.

Without the PR:

nrf52840dk_nrf52840: the filter finds matching nodes and build continues. Good.
nucleo_l073rz: twister fails with cmake dt build failure:
devicetree error: /chosen: undefined node label 'boot_partition'

nsim_sem: fails with cmake kconfig error:
warning: FLASH_MAP (defined at subsys/storage/flash_map/Kconfig:10) has direct dependencies FLASH_HAS_DRIVER_ENABLED with value n, but is currently being y-selected by the following symbols:

MCUBOOT_IMG_MANAGER (defined at subsys/dfu/Kconfig:25), with value y, direct dependencies (value: y), and select condition (value: y)

warning: FLASH_PAGE_LAYOUT (defined at drivers/flash/Kconfig:54) has direct dependencies FLASH_HAS_PAGE_LAYOUT && FLASH with value n, but is currently being y-selected by the following symbols:

STREAM_FLASH (defined at subsys/storage/stream/Kconfig:7), with value y, direct dependencies y (value: y)

warning: USE_DT_CODE_PARTITION (defined at Kconfig.zephyr:88) has direct dependencies HAS_FLASH_LOAD_OFFSET with value n, but is currently being y-selected by the following symbols:

BOOTLOADER_MCUBOOT (defined at Kconfig.zephyr:762), with value y, direct dependencies !MCUBOOT (value: y), and select condition !MCUBOOT (value: y)

error: Aborting due to Kconfig warnings

With this PR (since with it only dt is required for filtering):

INFO - 1/3 nsim_sem zephyr/tests/boot/test_mcuboot/boot.mcuboot SKIPPED (runtime filter)
INFO - 2/3 nucleo_l073rz zephyr/tests/boot/test_mcuboot/boot.mcuboot SKIPPED (runtime filter)
INFO - 3/3 nrf52840dk_nrf52840 zephyr/tests/boot/test_mcuboot/boot.mcuboot PASSED (build)

I also run a fuul scope (all platforms) locally on the PR and got such numbers:
141 of 517 passed, 25 failed 351 skipped 85% pass/(all-skip)
I guess still some work has to be done in clearing the failing platforms or adding them to test's platform_exclude, but this might be a way to go IMO

danieldegrasse · 2023-02-21T19:07:34Z

@PerMac I agree that #53499 is the way to go for MCUBoot support. I'm going to close this PR, since it seems that it's not the right direction for adding support to test MCUBoot

danieldegrasse added 3 commits October 19, 2022 09:14

danieldegrasse requested review from nashif, tejlmand, rerickson1, dcpleung, andyross, kgugala and pgielda as code owners October 19, 2022 14:50

zephyrbot added area: Build System area: RISCV RISCV Architecture (32-bit & 64-bit) platform: ESP32 Espressif ESP32 platform: Laird Connectivity Laird Connectivity labels Oct 19, 2022

zephyrbot requested review from carlocaione, edersondisouza, fkokosinski, glaubermaroto, greg-leach, jeremybettis, katsuster, LucasTambor, mgielda, sylvioalves, tgorochowik and uLipe and removed request for kgugala, rerickson1, tejlmand and dcpleung October 19, 2022 14:50

zephyrbot requested review from kgugala, rerickson1 and tejlmand and removed request for glaubermaroto, greg-leach and jeremybettis October 19, 2022 15:06

rerickson1 requested changes Oct 19, 2022

View reviewed changes

boards/arm/mg100/Kconfig.defconfig Show resolved Hide resolved

tejlmand requested changes Oct 31, 2022

View reviewed changes

github-actions bot added the Stale label Jan 22, 2023

github-actions bot removed the Stale label Jan 23, 2023

danieldegrasse closed this Feb 21, 2023

PerMac mentioned this pull request May 25, 2023

Tests: MCUboot testing: Extend to more targets #58122

Open

dleach02 deleted the feature/enable-mcuboot-test branch July 17, 2024 01:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: Enable MCUBoot test on additional platforms #51448

tests: Enable MCUBoot test on additional platforms #51448

danieldegrasse commented Oct 19, 2022

rerickson1 commented Oct 19, 2022 •

edited

Loading

danieldegrasse commented Oct 28, 2022

rerickson1 commented Oct 28, 2022

str4t0m commented Oct 28, 2022 •

edited

Loading

tejlmand commented Oct 31, 2022

tejlmand left a comment

danieldegrasse commented Nov 1, 2022

danieldegrasse commented Nov 1, 2022

rerickson1 commented Nov 1, 2022

tejlmand commented Nov 3, 2022

PerMac commented Nov 14, 2022

sylvioalves commented Nov 22, 2022

github-actions bot commented Jan 22, 2023

cfriedt commented Jan 22, 2023

sylvioalves commented Jan 22, 2023

PerMac commented Jan 24, 2023 •

edited

Loading

danieldegrasse commented Feb 21, 2023 •

edited

Loading

tests: Enable MCUBoot test on additional platforms #51448

tests: Enable MCUBoot test on additional platforms #51448

Conversation

danieldegrasse commented Oct 19, 2022

Introduction

Approach

Status of the PR

Issues to address

rerickson1 commented Oct 19, 2022 • edited Loading

danieldegrasse commented Oct 28, 2022

rerickson1 commented Oct 28, 2022

str4t0m commented Oct 28, 2022 • edited Loading

tejlmand commented Oct 31, 2022

tejlmand left a comment

Choose a reason for hiding this comment

danieldegrasse commented Nov 1, 2022

danieldegrasse commented Nov 1, 2022

rerickson1 commented Nov 1, 2022

tejlmand commented Nov 3, 2022

PerMac commented Nov 14, 2022

sylvioalves commented Nov 22, 2022

github-actions bot commented Jan 22, 2023

cfriedt commented Jan 22, 2023

sylvioalves commented Jan 22, 2023

PerMac commented Jan 24, 2023 • edited Loading

danieldegrasse commented Feb 21, 2023 • edited Loading

rerickson1 commented Oct 19, 2022 •

edited

Loading

str4t0m commented Oct 28, 2022 •

edited

Loading

PerMac commented Jan 24, 2023 •

edited

Loading

danieldegrasse commented Feb 21, 2023 •

edited

Loading